Python: feat(core): add max_duration_seconds bound and stop_reason signal to tool loop (#7587) - #7772
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
documentation
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Adds duration bounds and machine-readable termination reasons to Python function-invocation loops.
Changes:
- Adds and validates
max_duration_seconds. - Tracks stop reasons across streaming and non-streaming paths.
- Adds tests and changelog documentation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
python/packages/core/agent_framework/_tools.py |
Implements duration tracking and stop reasons. |
python/packages/core/tests/core/test_function_invocation_logic.py |
Tests duration limits and termination signals. |
python/CHANGELOG.md |
Documents the new behavior. |
Suppressed comments (2)
python/packages/core/agent_framework/_tools.py:3528
- The streaming path has the same enforcement gap: approved calls are replayed before this check, while the call-dropping/fallback logic at lines 3449-3466 recognizes only
max_function_calls. Consequently, an expired approval or a provider-emitted call despitetool_choice="none"can still execute. Include duration expiry in a shared pre-execution and fallback predicate.
if (
max_duration_seconds is not None
and (perf_counter() - budget_state["start_time"]) >= max_duration_seconds
):
python/packages/core/agent_framework/_tools.py:3518
- The streaming branch also leaks the internal action name
"stop"as a public stop reason. This is outside the documented value set and differs from approval-time error exhaustion, which reportscompleted. Use the same documented semantic reason for consecutive-error exhaustion in both paths.
budget_state.setdefault("stop_reason", "stop")
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
github-actions
Bot
changed the title
0ef8220 to
05ce001
Compare
GitHub Actions
Inactive
…eason to AgentResponse
- **agent-framework-core**: Refactored _apply_batch_limit_decision to compute perf_counter exactly once per decision point, eliminating the structural fragility where the threshold check and log message used separate clock samples. - **agent-framework-core**: Re-ordered limit checking in Phase 1 to execute before approval response resolution, successfully preventing execution during approved replays when limits are reached. A post-approval check ensures consecutive error limits (�ction == stop) remain handled. - **agent-framework-core**: Rewrote 6 tests in est_function_invocation_logic.py that used a fragile call_count mock. The tests now use a mutable clock array that advances directly during the tool execution semantic step, providing true robustness against internal engine refactors. Note: The fallback response trigger (_ensure_function_invocation_limit_fallback_response) remains scoped strictly to the function call limit, preserving pre-existing behavior. Expanding this to cover consecutive errors (�ction == stop) or duration timeouts is intentionally left out of scope for this fix.
… streaming limit decision, fix double-counted approval calls against max_function_calls
GitHub Actions
Inactive
GitHub Actions
Active
| # for streaming we recover it here from the inner ChatResponse stored in the closure. | ||
| if inner_chat_responses: | ||
| inner = inner_chat_responses[0] | ||
| stop_reason = inner.additional_properties.get("_agent_framework_stop_reason") |
There was a problem hiding this comment.
I'm not sure about this approach, is there a reason we can't use the finish_reason field, I wouldn't mind adding a custom reason into the existing FinishReasonLiteral (we would have to sync with Roger Barreto (@rogerbarreto) on a name for that and make it consistent between MEAI and here)
There was a problem hiding this comment.
The reason we went with a separate additional_properties key rather than repurposing finish_reason is that they seemed like two different things: finish_reason today reflects why one specific model call ended (from the provider), while this is about why the framework cut the whole multi-turn loop short — a decision we make, not the model. Folding it into finish_reason on the final response would mean losing whatever the model's own real finish_reason was on that last call.
That said, this is your call to make — happy to move this to FinishReasonLiteral if that's what you and Roger land on for consistency with MEAI. Let me know what name(s) you'd like and I'll wire it up accordingly.
There was a problem hiding this comment.
we had some internal discussion here, and we don't know what a user would do with this field. Updating finish_reason itself is also not ideal since that would be a breaking change in behavior. But we also do not like adding a lot of stuff into additional properties. So the question then becomes what would be the action the user needs to do that he needs this field? and are there other ways they could achieve the same?
GitHub Actions
Active
GitHub Actions
Active
Motivation & Context
Function invocation loops in
FunctionInvocationLayer(_tools.py) currently allow capping LLM roundtrips viamax_iterationsand total function calls viamax_function_calls, but lack a wall-clock time limit. Unattended or complex agent runs can execute tools repeatedly and stall for long periods without a bounded total duration.Additionally, callers currently have no programmatic way to determine why a function invocation run ended (e.g. normal completion vs hitting
max_iterationsor a tool limit).This PR addresses #7587 by introducing
max_duration_secondstoFunctionInvocationConfigurationand surfacing a_agent_framework_stop_reasonsignal onChatResponse.additional_properties.The issue's motivating scenario is a single, continuous, unattended loop execution. This PR implements that core case, and additionally extends the duration bound to persist across human-approval round-trips (see "Beyond the original ask" below) — an extension we made, not something #7587 explicitly requested.
Description & Review Guide
What are the major changes?
max_duration_secondsConfig Field: Addedmax_duration_seconds: float | NonetoFunctionInvocationConfiguration(TypedDict) and normalized validation (> 0orNone).max_duration_secondsis exceeded mid-loop (checked after each tool batch), further tool calls are disabled (tool_choice = "none") and the model is forced to produce a final text response, reusing the establishedmax_function_callsdegradation path.stop_reasonSignal: Surface_agent_framework_stop_reasoninChatResponse.additional_properties(and onAgentResponseforAgent.run(stream=True)) with values"completed","max_iterations","max_duration_seconds","max_function_calls", and"max_consecutive_errors"._apply_batch_limit_decisionhelper decides the stop reason and tool-disable state for both streaming and non-streaming loops, so the two paths can't independently disagree on precedence (duration → consecutive-errors → call-count).ResponseStreamwith_finalize_with_stop_reasonso streaming callers receive_agent_framework_stop_reasonon the finalChatResponsewithout altering individual streaming update counts.Beyond the original ask
budget_statenow persists inAgentSession.state(viaToolApprovalMiddleware) so duration is measured cumulatively even when a run pauses for human approval and resumes in a separateagent.run()call. #7587 motivating case has no approval step — this is our own generalization, not an explicit ask. Flagging it as the main thing worth a scoping decision: happy to split it into a follow-up PR if a single-execution-only bound is preferred for this one.What is the impact of these changes?
response.additional_properties["_agent_framework_stop_reason"]to programmatically handle how a run concluded.max_duration_secondsisNone(unlimited); approval-persistence logic is a no-op for callers not usingToolApprovalMiddleware.What do you want reviewers to focus on?
_agent_framework_stop_reasondesign vs. extendingFinishReasonLiteral._apply_batch_limit_decisionprecedence order when multiple bounds are hit in the same batch.Related Issue
Fixes #7587 (core duration bound and stop-reason signal); does not implement the token-count bound also proposed in that issue.
Contribution Checklist